Improve performance of placeholder and add ghs performance test.
#17589
Conversation
There was a problem hiding this comment.
I went through this whole PR and there is literally just one real change I can see -- you removed one of the Array.from(). I wonder what else impacted the performance, as I can see that you moved around some stuff, and you also merged two loops into one (not sure how much performance improvement this could be), but it seems that it's the same things being executed.
In addition to the two changes you have already described, there are two others that improve performance quite a bit. The first is adding In the document with 3000 images, the first improvement reduced the number of elements we iterated from 13,510,504 to 4,552,519. The second improvement further reduced this number to just 54,020. |
There was a problem hiding this comment.
I approve the code.
I wonder about this lazy loading though. Shouldn't we mark it as, at least, a minor breaking change? And maybe we could have this under a flag (opt-out if somebody does not like it for some reason)?
I think we should discuss it from product POV.
|
One thing I think I saw is a plugin or a customer implementation that would add those attributes (to be used on the publishing side). If we do it this way, it could lead to some confusing UIs: every image gains a flag that we think we set for publishing, and then it vanishes). |
|
To make sure we are on the same page — currently, this PR only adds the However, since we only add it to images with defined Which way should we go? |
|
I think that editing experience may be a breaking change too. We don't need to be hasty with this PR, let's discuss it. |
|
Just a note, as I may forget, what I mean is someone may have a feature that upcasts this to model. And you just triggered their potential feature on every image. I am not sure if with editing, but for example via clipboard pipeline or drag and drop. |
Suggested merge commit message (convention)
Other (ckeditor5): Add new
ghsperformance test.Other (engine): Improve performance of the placeholders.
Other (image): Add
loading="lazy"to images withheightandwidthattributes in editing view to improve loading performance.When looking at the numbers reported by the performance tests, we can see that these changes have reduced the load time of the
ghstest by around 23% (from 9400ms to 7300ms).However, when images were present in the document, a lot of code was run after they were loaded, so the browser window was still frozen for a few seconds after the editor was rendered. When this is considered, the performance is now 2 times better (loading, scripting, rendering, and painting took 17800ms before, but only 8900 ms after, including devtools overhead).